Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MOQT clientのSubscribe OKメッセージ送信対応 #58

Merged
merged 16 commits into from
Aug 19, 2024
Merged

MOQT clientのSubscribe OKメッセージ送信対応 #58

merged 16 commits into from
Aug 19, 2024

Conversation

tetter27
Copy link
Collaborator

@tetter27 tetter27 commented Jul 29, 2024 •
edited
Loading

内容

  • Subscribeメッセージを受信した際にSubscribeOKメッセージを返却する
    • (やっぱりエラーハンドリングは別のPRで実施します)
  • 変数名track_name_spaceをtrack_namespaceへ統合

議論ポイント

  • 現在sendSubscribeOkMessageをJS側においてある
  • エラーの検証をせずにSubscribeメッセージの内容をそのまま返す実装になっている
    • エラーの検証をするには実装をRust側へ移し、現在announceされているtrackを管理する必要がある
    • (管理のロジックを入れるのは結構大きな変更になるので先にコンセンサスを取りたい)

@test-coverage-report

This comment has been minimized.

Sign in to view
@tetter27
Copy link
Collaborator Author

  • moqt-client-sampleをsample部分とlib部分に分けてlib部分に管理を入れるのが理想
  • jsにTODOつけて実装しつつ、検討用のissueを立てる
    • sampleとlibにそれぞれどんな役割をもたせるか検討

@test-coverage-report

This comment has been minimized.

Sign in to view
@tetter27
Copy link
Collaborator Author

tetter27 commented Aug 1, 2024

issueは #60 として起票。

@test-coverage-report

This comment has been minimized.

Sign in to view
@test-coverage-report

This comment has been minimized.

Sign in to view
yuki-uchida
yuki-uchida reviewed Aug 5, 2024
View reviewed changes
js/main.js Outdated

// TODO: sendSubscribeErrorMessage
// TODO: Move error handling to lib.rs
announcedTrackNamespaces.forEach((announced) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

検索の際はfilter関数使った方が綺麗に見えると思います

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

というのも、for文はいろいろな形で使われるので、中身の条件分見ないとどういった処理をされているのかわからないのに対して、filter関数を使うと「フィルターしているんだな」ということがパッとわかるからです

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます。
今回は含まれるかどうかで判定したかったのでincludeを使ってみます。
もし意図が違ったら指摘お願いします。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OKです〜

@test-coverage-report

This comment has been minimized.

Sign in to view
@test-coverage-report
Copy link 

Filename                                                           Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
moqt-client-sample/src/lib.rs                                            3                 3     0.00%           3                 3     0.00%           5                 5     0.00%           0                 0         -
moqt-client-sample/src/utils.rs                                          1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
moqt-core/src/modules/constants.rs                                       1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
moqt-core/src/modules/handlers/announce_handler.rs                      14                14     0.00%           2                 2     0.00%          26                26     0.00%           0                 0         -
moqt-core/src/modules/handlers/server_setup_handler.rs                  35                 6    82.86%           7                 0   100.00%         104                 3    97.12%           0                 0         -
moqt-core/src/modules/handlers/subscribe_handler.rs                     17                17     0.00%           2                 2     0.00%          37                37     0.00%           0                 0         -
moqt-core/src/modules/handlers/unannounce_handler.rs                    14                14     0.00%           2                 2     0.00%          19                19     0.00%           0                 0         -
moqt-core/src/modules/handlers/unsubscribe_handler.rs                    8                 8     0.00%           2                 2     0.00%          15                15     0.00%           0                 0         -
moqt-core/src/modules/message_handler.rs                                13                13     0.00%           2                 2     0.00%          12                12     0.00%           0                 0         -
moqt-core/src/modules/message_type.rs                                   14                14     0.00%           5                 5     0.00%          12                12     0.00%           0                 0         -
moqt-core/src/modules/messages/announce_error_message.rs                28                 8    71.43%           6                 1    83.33%          84                 3    96.43%           0                 0         -
moqt-core/src/modules/messages/announce_message.rs                      37                 9    75.68%           9                 2    77.78%         154                 6    96.10%           0                 0         -
moqt-core/src/modules/messages/announce_ok_message.rs                   15                 3    80.00%           6                 1    83.33%          45                 3    93.33%           0                 0         -
moqt-core/src/modules/messages/client_setup_message.rs                  46                12    73.91%           6                 1    83.33%         120                 7    94.17%           0                 0         -
moqt-core/src/modules/messages/object_message.rs                        57                15    73.68%          12                 2    83.33%         234                 6    97.44%           0                 0         -
moqt-core/src/modules/messages/server_setup_message.rs                  27                 6    77.78%           6                 1    83.33%          87                 3    96.55%           0                 0         -
moqt-core/src/modules/messages/setup_parameters.rs                      57                11    80.70%          17                 1    94.12%         152                 6    96.05%           0                 0         -
moqt-core/src/modules/messages/subscribe_error_message.rs               27                27     0.00%           4                 4     0.00%          51                51     0.00%           0                 0         -
moqt-core/src/modules/messages/subscribe_ok_message.rs                  22                22     0.00%           4                 4     0.00%          41                41     0.00%           0                 0         -
moqt-core/src/modules/messages/subscribe_request_message.rs             65                65     0.00%           9                 9     0.00%         118               118     0.00%           0                 0         -
moqt-core/src/modules/messages/unannounce_message.rs                    12                12     0.00%           4                 4     0.00%          17                17     0.00%           0                 0         -
moqt-core/src/modules/messages/unsubscribe_message.rs                   19                19     0.00%           6                 6     0.00%          29                29     0.00%           0                 0         -
moqt-core/src/modules/messages/version_specific_parameters.rs           56                 9    83.93%          16                 1    93.75%         239                 4    98.33%           0                 0         -
moqt-core/src/modules/moqt_client.rs                                     9                 3    66.67%           6                 2    66.67%          27                 7    74.07%           0                 0         -
moqt-core/src/modules/server_processes/announce_message.rs              22                22     0.00%           2                 2     0.00%          32                32     0.00%           0                 0         -
moqt-core/src/modules/server_processes/client_setup_message.rs          15                15     0.00%           1                 1     0.00%          21                21     0.00%           0                 0         -
moqt-core/src/modules/server_processes/subscribe_message.rs             22                22     0.00%           2                 2     0.00%          40                40     0.00%           0                 0         -
moqt-core/src/modules/variable_bytes.rs                                 28                 5    82.14%           9                 0   100.00%         104                 8    92.31%           0                 0         -
moqt-core/src/modules/variable_integer.rs                               47                 1    97.87%          15                 0   100.00%         149                 1    99.33%           0                 0         -
moqt-server-sample/src/main.rs                                           6                 6     0.00%           3                 3     0.00%          31                31     0.00%           0                 0         -
moqt-server/src/lib.rs                                                 176               176     0.00%          22                22     0.00%         269               269     0.00%           0                 0         -
moqt-server/src/modules/buffer_manager.rs                               36                36     0.00%           4                 4     0.00%          62                62     0.00%           0                 0         -
moqt-server/src/modules/stream_manager.rs                               61                61     0.00%           7                 7     0.00%          78                78     0.00%           0                 0         -
moqt-server/src/modules/track_namespace_manager.rs                     416                53    87.26%          92                 0   100.00%        1134                13    98.85%           0                 0         -
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                                 1426               709    50.28%         295               100    66.10%        3550               987    72.20%           0                 0         -

Coverage report details

Base automatically changed from impl/subscriber_manage_cmd to master August 19, 2024 03:33
@yuki-uchida yuki-uchida merged commit 1124552 into master Aug 19, 2024
3 checks passed
@yuki-uchida yuki-uchida deleted the impl/subscribe_ok branch August 19, 2024 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuki-uchida yuki-uchida yuki-uchida approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tetter27 @yuki-uchida